Skip to content

Conversation

farzonl
Copy link
Member

@farzonl farzonl commented Aug 15, 2025

fixes #152348

SimplifyCFG collapses raw buffer store from a if\else load into a select. This change duplicates the instructions from the load to the store back into seperate blocks.

fixes llvm#152348

SimplifyCFG collapses raw buffer store from a if\else load into a
select. This change duplicates the instructions from the load to the
store back into seperate blocks.
@llvmbot
Copy link
Member

llvmbot commented Aug 15, 2025

@llvm/pr-subscribers-backend-directx

Author: Farzon Lotfi (farzonl)

Changes

fixes #152348

SimplifyCFG collapses raw buffer store from a if\else load into a select. This change duplicates the instructions from the load to the store back into seperate blocks.


Full diff: https://github.com/llvm/llvm-project/pull/153858.diff

2 Files Affected:

  • (modified) llvm/lib/Target/DirectX/DXILLegalizePass.cpp (+90)
  • (added) llvm/test/CodeGen/DirectX/issue-152348.ll (+40)
diff --git a/llvm/lib/Target/DirectX/DXILLegalizePass.cpp b/llvm/lib/Target/DirectX/DXILLegalizePass.cpp
index 3427968d199f6..8391ec2d332be 100644
--- a/llvm/lib/Target/DirectX/DXILLegalizePass.cpp
+++ b/llvm/lib/Target/DirectX/DXILLegalizePass.cpp
@@ -15,8 +15,11 @@
 #include "llvm/IR/InstIterator.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/IntrinsicInst.h"
+#include "llvm/IR/IntrinsicsDirectX.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Pass.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include <functional>
 
@@ -622,6 +625,92 @@ legalizeScalarLoadStoreOnArrays(Instruction &I,
   return true;
 }
 
+// Note: Legalization to undo SimplifyCFG. Ideally, SimplifyCFG's
+// TargetTransformInfo would ignore our resource intrinsics, but
+// it doesn't. This Works for a single select; multiple selects on
+// raw buffer loads won’t be legalized. The hasOneUser for ExtractValueInst
+// of dx_resource_load_rawbuffer is enforced in DXILOpLowering,
+// so checking it here is fine.
+static bool
+legalizeBuffLoadSelectCalls(Instruction &I,
+                            SmallVectorImpl<Instruction *> &ToRemove,
+                            DenseMap<Value *, Value *> &) {
+  // Check if this is a dx_resource_load_rawbuffer intrinsic
+  auto *II = dyn_cast<IntrinsicInst>(&I);
+  if (!II || II->getIntrinsicID() != Intrinsic::dx_resource_load_rawbuffer)
+    return false;
+
+  // Check if the first argument is a select instruction
+  Value *Arg0 = II->getArgOperand(0);
+  auto *Sel = dyn_cast<SelectInst>(Arg0);
+  if (!Sel)
+    return false;
+
+  if (!II->hasOneUser())
+    return false;
+  ExtractValueInst *EVI = dyn_cast<ExtractValueInst>(*II->user_begin());
+  if (!EVI->hasOneUser())
+    return false;
+  auto *StoreII = dyn_cast<IntrinsicInst>(*EVI->user_begin());
+  if (!StoreII ||
+      StoreII->getIntrinsicID() != Intrinsic::dx_resource_store_rawbuffer)
+    return false;
+
+  BasicBlock *BB = II->getParent();
+  Function *F = BB->getParent();
+  IRBuilder<> Builder(StoreII);
+
+  // Create new basic blocks
+  BasicBlock *ThenBB = BasicBlock::Create(F->getContext(), "rawbuf.if", F);
+  BasicBlock *ElseBB = BasicBlock::Create(F->getContext(), "rawbuf.else", F);
+  BasicBlock *ContinueBB =
+      BB->splitBasicBlock(std::next(II->getIterator()), "rawbuf.continue");
+
+  // Remove the unconditional branch
+  BB->getTerminator()->eraseFromParent();
+
+  // Create a conditional branch based on the select condition
+  Builder.SetInsertPoint(BB);
+  Builder.CreateCondBr(Sel->getCondition(), ThenBB, ElseBB);
+
+  // Create the true path
+  Builder.SetInsertPoint(ThenBB);
+  Instruction *TrueResourceLoad = II->clone();
+  Instruction *TrueExtract = EVI->clone();
+  Instruction *TrueResourceStore = StoreII->clone();
+  TrueResourceLoad->setOperand(0, Sel->getTrueValue());
+  TrueExtract->setOperand(0, TrueResourceLoad);
+  TrueResourceStore->setOperand(3, TrueExtract);
+  Builder.Insert(TrueResourceLoad);
+  Builder.Insert(TrueExtract);
+  Builder.Insert(TrueResourceStore);
+  Builder.CreateBr(ContinueBB);
+
+  // Create the false path
+  Builder.SetInsertPoint(ElseBB);
+  Instruction *FalseResourceLoad = II->clone();
+  Instruction *FalseExtract = EVI->clone();
+  Instruction *FalseResourceStore = StoreII->clone();
+  FalseResourceLoad->setOperand(0, Sel->getFalseValue());
+  FalseExtract->setOperand(0, FalseResourceLoad);
+  FalseResourceStore->setOperand(3, FalseExtract);
+  Builder.Insert(FalseResourceLoad);
+  Builder.Insert(FalseExtract);
+  Builder.Insert(FalseResourceStore);
+  Builder.CreateBr(ContinueBB);
+
+  // Set up the merge block
+  Builder.SetInsertPoint(ContinueBB);
+
+  // Mark the instructions for removal
+  ToRemove.push_back(Sel);
+  ToRemove.push_back(II);
+  ToRemove.push_back(EVI);
+  ToRemove.push_back(StoreII);
+
+  return true;
+}
+
 namespace {
 class DXILLegalizationPipeline {
 
@@ -671,6 +760,7 @@ class DXILLegalizationPipeline {
     LegalizationPipeline[Stage2].push_back(
         downcastI64toI32InsertExtractElements);
     LegalizationPipeline[Stage2].push_back(legalizeScalarLoadStoreOnArrays);
+    LegalizationPipeline[Stage2].push_back(legalizeBuffLoadSelectCalls);
   }
 };
 
diff --git a/llvm/test/CodeGen/DirectX/issue-152348.ll b/llvm/test/CodeGen/DirectX/issue-152348.ll
new file mode 100644
index 0000000000000..92e7b7bfa727e
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/issue-152348.ll
@@ -0,0 +1,40 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes='dxil-legalize' -mtriple=dxil-pc-shadermodel6.3-library %s | FileCheck %s
+
+@default_int= global i32 0
+@.str = internal unnamed_addr constant [2 x i8] c"a\00", align 1
+
+define void @CSMain() local_unnamed_addr {
+; CHECK-LABEL: define void @CSMain() local_unnamed_addr {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[TMP0:%.*]] = tail call target("dx.RawBuffer", half, 1, 0) @llvm.dx.resource.handlefromimplicitbinding.tdx.RawBuffer_f16_1_0t(i32 0, i32 0, i32 1, i32 0, i1 false, ptr nonnull @.str)
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call target("dx.RawBuffer", half, 1, 0) @llvm.dx.resource.handlefromimplicitbinding.tdx.RawBuffer_f16_1_0t(i32 1, i32 0, i32 1, i32 0, i1 false, ptr nonnull @.str)
+; CHECK-NEXT:    [[TMP2:%.*]] = tail call target("dx.RawBuffer", half, 1, 0) @llvm.dx.resource.handlefromimplicitbinding.tdx.RawBuffer_f16_1_0t(i32 2, i32 0, i32 1, i32 0, i1 false, ptr nonnull @.str)
+; CHECK-NEXT:    [[INT:%.*]] = load i32, ptr @default_int, align 4
+; CHECK-NEXT:    [[TOBOOL_NOT_I:%.*]] = icmp eq i32 [[INT]], 0
+; CHECK-NEXT:    br i1 [[TOBOOL_NOT_I]], label %[[RAWBUF_IF:.*]], label %[[RAWBUF_ELSE:.*]]
+; CHECK:       [[RAWBUF_CONTINUE:.*]]:
+; CHECK-NEXT:    ret void
+; CHECK:       [[RAWBUF_IF]]:
+; CHECK-NEXT:    [[TMP3:%.*]] = call { half, i1 } @llvm.dx.resource.load.rawbuffer.f16.tdx.RawBuffer_f16_1_0t(target("dx.RawBuffer", half, 1, 0) [[TMP0]], i32 0, i32 0)
+; CHECK-NEXT:    [[TMP4:%.*]] = extractvalue { half, i1 } [[TMP3]], 0
+; CHECK-NEXT:    call void @llvm.dx.resource.store.rawbuffer.tdx.RawBuffer_f16_1_0t.f16(target("dx.RawBuffer", half, 1, 0) [[TMP2]], i32 0, i32 0, half [[TMP4]])
+; CHECK-NEXT:    br label %[[RAWBUF_CONTINUE]]
+; CHECK:       [[RAWBUF_ELSE]]:
+; CHECK-NEXT:    [[TMP5:%.*]] = call { half, i1 } @llvm.dx.resource.load.rawbuffer.f16.tdx.RawBuffer_f16_1_0t(target("dx.RawBuffer", half, 1, 0) [[TMP1]], i32 0, i32 0)
+; CHECK-NEXT:    [[TMP6:%.*]] = extractvalue { half, i1 } [[TMP5]], 0
+; CHECK-NEXT:    call void @llvm.dx.resource.store.rawbuffer.tdx.RawBuffer_f16_1_0t.f16(target("dx.RawBuffer", half, 1, 0) [[TMP2]], i32 0, i32 0, half [[TMP6]])
+; CHECK-NEXT:    br label %[[RAWBUF_CONTINUE]]
+;
+entry:
+  %0 = tail call target("dx.RawBuffer", half, 1, 0) @llvm.dx.resource.handlefromimplicitbinding.tdx.RawBuffer_f16_1_0t(i32 0, i32 0, i32 1, i32 0, i1 false, ptr nonnull @.str)
+  %1 = tail call target("dx.RawBuffer", half, 1, 0) @llvm.dx.resource.handlefromimplicitbinding.tdx.RawBuffer_f16_1_0t(i32 1, i32 0, i32 1, i32 0, i1 false, ptr nonnull @.str)
+  %2 = tail call target("dx.RawBuffer", half, 1, 0) @llvm.dx.resource.handlefromimplicitbinding.tdx.RawBuffer_f16_1_0t(i32 2, i32 0, i32 1, i32 0, i1 false, ptr nonnull @.str)
+  %int = load i32, ptr @default_int
+  %tobool.not.i = icmp eq i32 %int, 0
+  %.rawbuffer = select i1 %tobool.not.i, target("dx.RawBuffer", half, 1, 0) %0, target("dx.RawBuffer", half, 1, 0) %1
+  %4 = call { half, i1 } @llvm.dx.resource.load.rawbuffer.f16.tdx.RawBuffer_f16_1_0t(target("dx.RawBuffer", half, 1, 0) %.rawbuffer, i32 0, i32 0)
+  %5 = extractvalue { half, i1 } %4, 0
+  call void @llvm.dx.resource.store.rawbuffer.tdx.RawBuffer_f16_1_0t.f16(target("dx.RawBuffer", half, 1, 0) %2, i32 0, i32 0, half %5)
+  ret void
+}

@@ -622,6 +625,92 @@ legalizeScalarLoadStoreOnArrays(Instruction &I,
return true;
}

// Note: Legalization to undo SimplifyCFG. Ideally, SimplifyCFG's
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: fix comment grammar, make shorter.

@alsepkow
Copy link
Contributor

alsepkow commented Aug 15, 2025

LGTM! Assuming the grammar fixups :)

if (!Sel)
return false;

if (!II->hasOneUser())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this check necessary?

Copy link
Member Author

@farzonl farzonl Aug 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replaceResRetUses requires that ExtractValueInst be the only use. If it has more than this then it isn't the pattern we are trying to legalize.

assert(OldResult->hasOneUse() &&
isa<ExtractValueInst>(*OldResult->user_begin()) &&

Builder.CreateBr(ContinueBB);

// Set up the merge block
Builder.SetInsertPoint(ContinueBB);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that we want to set the insert point at the continue BB. But the position of the continue BB is before that of the If and Then BB's. Why, was this deliberate?
I'd expect the Continue BB to be positioned after the if and then BBs.
I had also thought that this wouldn't be possible since labels need to be declared ahead of time, but the select instruction references the if / then labels without forward declaration, so it should be possible to put the continue block after the then / else blocks, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually we don't need to do Builder.SetInsertPoint(ContinueBB); because we aren't adding any instructions to ContinueBB. so will delete this.

The if and else basic blocks can be anywhere we want them to be because we aren't doing any fall through cases. Each basic block ends with Builder.CreateBr(ContinueBB); What you are likely thinking of is maybe removing the unconditional branch in the else case and letting that fall through to the ContinueBB basic block. I suppose that could be an optimization, but it isn't required for correctness.

Not sure what you are talking about with forward declares. We do define the If\Else\Continue basic blocks before we do anything else.

@nikic
Copy link
Contributor

nikic commented Aug 16, 2025

Is there a general constraint that target("dx.RawBuffer") cannot be used in select or just with these intrinsics?

I'm wondering if we should be preventing this from happening in the first place if it's a legality constraint. We have

bool llvm::canReplaceOperandWithVariable(const Instruction *I, unsigned OpIdx) {
to control this and could come up with a way to support target types or target intrinsics in there.

@bogner
Copy link
Contributor

bogner commented Aug 18, 2025

Is there a general constraint that target("dx.RawBuffer") cannot be used in select or just with these intrinsics?

I'm wondering if we should be preventing this from happening in the first place if it's a legality constraint. We have

bool llvm::canReplaceOperandWithVariable(const Instruction *I, unsigned OpIdx) {

to control this and could come up with a way to support target types or target intrinsics in there.

This is an interesting idea. Are you thinking that we'd do something like add another property to TargetExtType::hasProperty so that target types could opt in/out of canReplaceOperandWithVariable (or possibly more generally say it can't be replaced with a non-constant value)?

@nikic
Copy link
Contributor

nikic commented Aug 18, 2025

Is there a general constraint that target("dx.RawBuffer") cannot be used in select or just with these intrinsics?
I'm wondering if we should be preventing this from happening in the first place if it's a legality constraint. We have

bool llvm::canReplaceOperandWithVariable(const Instruction *I, unsigned OpIdx) {

to control this and could come up with a way to support target types or target intrinsics in there.

This is an interesting idea. Are you thinking that we'd do something like add another property to TargetExtType::hasProperty so that target types could opt in/out of canReplaceOperandWithVariable (or possibly more generally say it can't be replaced with a non-constant value)?

Depends on what the exact requirements are. I think the options here would be:

  1. Add a TargetExtType property that forbids use of the type in select/phi (and presumably non-intrinsic calls?) Basically, to give the target type the same constraints as a token type.
  2. Add an attribute similar to immarg which can be used to prevent introduction of select/phi for an intrinsic argument -- but without limiting to constant integer arguments.

@farzonl
Copy link
Member Author

farzonl commented Aug 19, 2025

Is there a general constraint that target("dx.RawBuffer") cannot be used in select or just with these intrinsics?
I'm wondering if we should be preventing this from happening in the first place if it's a legality constraint. We have

bool llvm::canReplaceOperandWithVariable(const Instruction *I, unsigned OpIdx) {

to control this and could come up with a way to support target types or target intrinsics in there.

This is an interesting idea. Are you thinking that we'd do something like add another property to TargetExtType::hasProperty so that target types could opt in/out of canReplaceOperandWithVariable (or possibly more generally say it can't be replaced with a non-constant value)?

Depends on what the exact requirements are. I think the options here would be:

  1. Add a TargetExtType property that forbids use of the type in select/phi (and presumably non-intrinsic calls?) Basically, to give the target type the same constraints as a token type.
  2. Add an attribute similar to immarg which can be used to prevent introduction of select/phi for an intrinsic argument -- but without limiting to constant integer arguments.

I'm not sure this is going to work out for us.
CC: @bogner

So didn't want to invest all the plumbing to get this working just for RawBuffers so keeping the catch all broad for now. Net result was this experiment

diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index ac344904f90f..c0995bf6346e 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -3852,6 +3852,9 @@ bool llvm::canReplaceOperandWithVariable(const Instruction *I, unsigned OpIdx) {
   if (I->isLifetimeStartOrEnd())
     return false;
 
+  if (Op->getType()->isTargetExtTy())
+    return false;
+
   // Early exit.
   if (!isa<Constant, InlineAsm>(Op))
     return true;

This does prevent the select from happening

define void @CSMain() local_unnamed_addr #1 {
entry:
  %d.cb_h.i.i = tail call target("dx.CBuffer", target("dx.Layout", %__cblayout_d, 16, 0, 4, 8, 12)) @llvm.dx.resource.handlefromimplicitbinding.tdx.CBuffer_tdx.Layout_s___cblayout_ds_16_0_4_8_12tt(i32 3, i32 0, i32 1, i32 0, i1 false, ptr nonnull @d.str)
  store target("dx.CBuffer", target("dx.Layout", %__cblayout_d, 16, 0, 4, 8, 12)) %d.cb_h.i.i, ptr @d.cb, align 4
  %0 = load i32, ptr addrspace(2) @h, align 4, !tbaa !6
  %tobool.not.i = icmp eq i32 %0, 0
  %1 = load i32, ptr addrspace(2) @g, align 4, !tbaa !6
  br i1 %tobool.not.i, label %if.else.i, label %if.then.i

if.then.i:                                        ; preds = %entry
  %2 = tail call target("dx.RawBuffer", half, 1, 0) @llvm.dx.resource.handlefromimplicitbinding.tdx.RawBuffer_f16_1_0t(i32 1, i32 0, i32 1, i32 0, i1 false, ptr nonnull @.str.2)
  %3 = load i32, ptr addrspace(2) @f, align 4, !tbaa !6
  %4 = tail call noundef nonnull align 2 dereferenceable(2) ptr @llvm.dx.resource.getpointer.p0.tdx.RawBuffer_f16_1_0t(target("dx.RawBuffer", half, 1, 0) %2, i32 %3)
  br label %_Z6CSMainv.exit

if.else.i:                                        ; preds = %entry
  %5 = tail call target("dx.RawBuffer", half, 1, 0) @llvm.dx.resource.handlefromimplicitbinding.tdx.RawBuffer_f16_1_0t(i32 0, i32 0, i32 1, i32 0, i1 false, ptr nonnull @.str)
  %6 = load i32, ptr addrspace(2) @e, align 4, !tbaa !6
  %7 = tail call noundef nonnull align 2 dereferenceable(2) ptr @llvm.dx.resource.getpointer.p0.tdx.RawBuffer_f16_1_0t(target("dx.RawBuffer", half, 1, 0) %5, i32 %6)
  br label %_Z6CSMainv.exit

_Z6CSMainv.exit:                                  ; preds = %if.then.i, %if.else.i
  %.sink1 = phi ptr [ %4, %if.then.i ], [ %7, %if.else.i ]
  %8 = tail call target("dx.RawBuffer", half, 1, 0) @llvm.dx.resource.handlefromimplicitbinding.tdx.RawBuffer_f16_1_0t(i32 2, i32 0, i32 1, i32 0, i1 false, ptr nonnull @.str.4)
  %9 = tail call noundef nonnull align 2 dereferenceable(2) ptr @llvm.dx.resource.getpointer.p0.tdx.RawBuffer_f16_1_0t(target("dx.RawBuffer", half, 1, 0) %8, i32 %1)
  %10 = load half, ptr %.sink1, align 2, !tbaa !10
  store half %10, ptr %9, align 2, !tbaa !10
  ret void
}

But now we are blowing up in DXILResourceAccess.cpp:234 instead of DXILOpLowering.cpp

llvm_unreachable("Unhandled instruction - pointer escaped?");

Unhandled instruction - pointer escaped?
UNREACHABLE executed at /Users/farzonlotfi/Projects/llvm-project/llvm/lib/Target/DirectX/DXILResourceAccess.cpp:234!
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: /Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-dxc test.hlsl -E CSMain -T cs_6_2 -enable-16bit-types -O3
1.      <eof> parser at end of file
2.      Code generation
3.      Running pass 'Function Pass Manager' on module 'test.hlsl'.
4.      Running pass 'DXIL Resource Access' on function '@CSMain'
 #0 0x0000000105721cc0 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x103381cc0)
 #1 0x00000001057222e0 PrintStackTraceSignalHandler(void*) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x1033822e0)
 #2 0x000000010571fb7c llvm::sys::RunSignalHandlers() (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x10337fb7c)
 #3 0x0000000105721494 llvm::sys::CleanupOnSignal(unsigned long) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x103381494)
 #4 0x00000001055bf0f8 (anonymous namespace)::CrashRecoveryContextImpl::HandleCrash(int, unsigned long) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x10321f0f8)
 #5 0x00000001055bf5dc CrashRecoverySignalHandler(int) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x10321f5dc)
 #6 0x00000001861c16a4 (/usr/lib/system/libsystem_platform.dylib+0x1804ad6a4)
 #7 0x000000018618788c (/usr/lib/system/libsystem_pthread.dylib+0x18047388c)
 #8 0x0000000186090a3c (/usr/lib/system/libsystem_c.dylib+0x18037ca3c)
 #9 0x00000001055d661c llvm::install_out_of_memory_new_handler() (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x10323661c)
#10 0x00000001024d0934 replaceAccess(llvm::IntrinsicInst*, llvm::dxil::ResourceTypeInfo&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x100130934)
#11 0x00000001024d029c transformResourcePointers(llvm::Function&, llvm::DXILResourceTypeMap&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x10013029c)
#12 0x00000001024d3238 (anonymous namespace)::DXILResourceAccessLegacy::runOnFunction(llvm::Function&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x100133238)
#13 0x000000010440a1c8 llvm::FPPassManager::runOnFunction(llvm::Function&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x10206a1c8)
#14 0x0000000104410d20 llvm::FPPassManager::runOnModule(llvm::Module&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x102070d20)
#15 0x000000010440aa4c (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x10206aa4c)
#16 0x000000010440a5d0 llvm::legacy::PassManagerImpl::run(llvm::Module&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x10206a5d0)
#17 0x0000000104411128 llvm::legacy::PassManager::run(llvm::Module&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x102071128)
#18 0x00000001060e6738 (anonymous namespace)::EmitAssemblyHelper::RunCodegenPipeline(clang::BackendAction, std::__1::unique_ptr<llvm::raw_pwrite_stream, std::__1::default_delete<llvm::raw_pwrite_stream>>&, std::__1::unique_ptr<llvm::ToolOutputFile, std::__1::default_delete<llvm::ToolOutputFile>>&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x103d46738)
#19 0x00000001060d6d40 (anonymous namespace)::EmitAssemblyHelper::emitAssembly(clang::BackendAction, std::__1::unique_ptr<llvm::raw_pwrite_stream, std::__1::default_delete<llvm::raw_pwrite_stream>>, clang::BackendConsumer*) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x103d36d40)
#20 0x00000001060d6234 clang::emitBackendOutput(clang::CompilerInstance&, clang::CodeGenOptions&, llvm::StringRef, llvm::Module*, clang::BackendAction, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>, std::__1::unique_ptr<llvm::raw_pwrite_stream, std::__1::default_delete<llvm::raw_pwrite_stream>>, clang::BackendConsumer*) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x103d36234)
#21 0x0000000106878da0 clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x1044d8da0)
#22 0x000000010917b16c clang::ParseAST(clang::Sema&, bool, bool) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x106ddb16c)
#23 0x000000010743b954 clang::ASTFrontendAction::ExecuteAction() (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x10509b954)
#24 0x000000010687e8c4 clang::CodeGenAction::ExecuteAction() (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x1044de8c4)
#25 0x000000010743b164 clang::FrontendAction::Execute() (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x10509b164)
#26 0x0000000107350930 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x104fb0930)
#27 0x00000001075836b8 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x1051e36b8)
#28 0x00000001023b4630 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x100014630)
#29 0x00000001023a6858 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&, llvm::ToolContext const&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x100006858)
#30 0x00000001023b303c clang_main(int, char**, llvm::ToolContext const&)::$_0::operator()(llvm::SmallVectorImpl<char const*>&) const (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x10001303c)
#31 0x00000001023b300c int llvm::function_ref<int (llvm::SmallVectorImpl<char const*>&)>::callback_fn<clang_main(int, char**, llvm::ToolContext const&)::$_0>(long, llvm::SmallVectorImpl<char const*>&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x10001300c)
#32 0x0000000107049e10 llvm::function_ref<int (llvm::SmallVectorImpl<char const*>&)>::operator()(llvm::SmallVectorImpl<char const*>&) const (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x104ca9e10)
#33 0x0000000107049dd0 clang::driver::CC1Command::Execute(llvm::ArrayRef<std::__1::optional<llvm::StringRef>>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>*, bool*) const::$_0::operator()() const (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x104ca9dd0)
#34 0x0000000107049d9c void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<std::__1::optional<llvm::StringRef>>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>*, bool*) const::$_0>(long) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x104ca9d9c)
#35 0x00000001035e6538 llvm::function_ref<void ()>::operator()() const (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x101246538)
#36 0x00000001055bef48 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x10321ef48)
#37 0x00000001070462e4 clang::driver::CC1Command::Execute(llvm::ArrayRef<std::__1::optional<llvm::StringRef>>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>*, bool*) const (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x104ca62e4)
#38 0x0000000106fd8d38 clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x104c38d38)
#39 0x0000000106fd8fc8 clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::__1::pair<int, clang::driver::Command const*>>&, bool) const (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x104c38fc8)
#40 0x0000000106ff6440 clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::__1::pair<int, clang::driver::Command const*>>&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x104c56440)
#41 0x00000001023a5fdc clang_main(int, char**, llvm::ToolContext const&) (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x100005fdc)
#42 0x00000001023e1758 main (/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-20+0x100041758)
#43 0x0000000185de6b98
clang-dxc: error: clang frontend command failed with exit code 134 (use -v to see invocation)
clang version 22.0.0git ([email protected]:llvm/llvm-project.git 86e23af6be1576fb3e5a6aeb1c59afc7026133d7)
Target: dxilv1.2-unknown-shadermodel6.2-compute
Thread model: posix
InstalledDir: /Users/farzonlotfi/Projects/llvm_debug_build/bin
Build config: +unoptimized, +assertions
clang-dxc: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang-dxc: note: diagnostic msg: /var/folders/c5/8k67fxsn7gq5d835vtqt_rcm0000gn/T/test-c6a74e.hlsl
clang-dxc: note: diagnostic msg: /var/folders/c5/8k67fxsn7gq5d835vtqt_rcm0000gn/T/test-c6a74e.sh
clang-dxc: note: diagnostic msg: Crash backtrace is located in
clang-dxc: note: diagnostic msg: /Users/farzonlotfi/Library/Logs/DiagnosticReports/clang-20_<YYYY-MM-DD-HHMMSS>_<hostname>.crash
clang-dxc: note: diagnostic msg: (choose the .crash file that corresponds to your crash)
clang-dxc: note: diagnostic msg: 

********************
farzonlotfi@mac llvm-project % lldb -- /Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-dxc test.hlsl -E CSMain -T cs_6_2 -enable-16bit-types -O3                     
(lldb) target create "/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-dxc"
Current executable set to '/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-dxc' (arm64).
(lldb) settings set -- target.run-args  "test.hlsl" "-E" "CSMain" "-T" "cs_6_2" "-enable-16bit-types" "-O3"
(lldb) b llvm/lib/Target/DirectX/DXILResourceAccess.cpp:234
Breakpoint 1: where = clang-dxc`replaceAccess(llvm::IntrinsicInst*, llvm::dxil::ResourceTypeInfo&) + 724 at DXILResourceAccess.cpp:234:7, address = 0x000000010013091c
(lldb) run
Process 6926 launched: '/Users/farzonlotfi/Projects/llvm_debug_build/bin/clang-dxc' (arm64)
Process 6926 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
    frame #0: 0x000000010013091c clang-dxc`replaceAccess(II=0x000000012b829e40, RTI=0x000000012b82e218) at DXILResourceAccess.cpp:234:7
   231        DeadInsts.push_back(LI);
   232 
   233      } else
-> 234        llvm_unreachable("Unhandled instruction - pointer escaped?");
   235    }
   236 
   237    // Traverse the now-dead instructions in RPO and remove them.

What is happening is replaceAccess is only designed to support loads, stores, and geps, and now we are introducing a phi node

lldb) expr II->dump()
  %4 = tail call noundef nonnull align 2 dereferenceable(2) ptr @llvm.dx.resource.getpointer.p0.tdx.RawBuffer_f16_1_0t(target("dx.RawBuffer", half, 1, 0) %2, i32 %3)
(lldb) expr Current.Access->dump()
  %.sink1 = phi ptr [ %4, %if.then.i ], [ %7, %if.else.i ]

This is actually a similar failure we were having when we were just trying to put the call instructions into the if and else basic blocks. To fix this issue the whole chain from load to store needs to be seperate.

@farzonl
Copy link
Member Author

farzonl commented Aug 19, 2025

Is there a general constraint that target("dx.RawBuffer") cannot be used in select or just with these intrinsics?
I'm wondering if we should be preventing this from happening in the first place if it's a legality constraint. We have

bool llvm::canReplaceOperandWithVariable(const Instruction *I, unsigned OpIdx) {

to control this and could come up with a way to support target types or target intrinsics in there.

This is an interesting idea. Are you thinking that we'd do something like add another property to TargetExtType::hasProperty so that target types could opt in/out of canReplaceOperandWithVariable (or possibly more generally say it can't be replaced with a non-constant value)?

Depends on what the exact requirements are. I think the options here would be:

  1. Add a TargetExtType property that forbids use of the type in select/phi (and presumably non-intrinsic calls?) Basically, to give the target type the same constraints as a token type.
  2. Add an attribute similar to immarg which can be used to prevent introduction of select/phi for an intrinsic argument -- but without limiting to constant integer arguments.

This solution might still have legs, but will require changing assumptions about how we handle Resource Types in some of our backend passes. I will investigate and move this pr to draft mode for now.

@farzonl
Copy link
Member Author

farzonl commented Aug 21, 2025

closing since #154620 supersedes this change.

@farzonl farzonl closed this Aug 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DirectX] Select of two different RawBuffer can not easily be reduced to a callInst.
6 participants